Use ONNX Frontend instead of ONNX Reader #7031
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mbencer
requested review from
postrational,
ilyachur,
tomdol,
slyalin and
nosovmik
postrational
approved these changes
Aug 11, 2021
ilya-lavrenov
approved these changes
Aug 12, 2021
ilyachur
approved these changes
Aug 13, 2021
tomdol
approved these changes
Aug 16, 2021
Co-authored-by: Tatiana Savina <[email protected]>
…o mbencer/LoadByModel
ilyachur
reviewed
Aug 24, 2021
| @@ -282,6 +270,19 @@ CNNNetwork details::ReadNetwork(const std::string& model, | |||
| return reader->read(modelStream, exts); | |||
| } | |||
| } | |||
| // Try to load with FrontEndManager | |||
| // NOTE: weights argument is ignored | |||
| static ngraph::frontend::FrontEndManager manager; | |||
There was a problem hiding this comment.
Can we use one instance of FrontEndManager for core object?
Now you have 2 static instances
| @@ -259,7 +247,7 @@ CNNNetwork details::ReadNetwork(const std::string& modelPath, | |||
| } | |||
| if (inputModel) { | |||
| auto ngFunc = FE->convert(inputModel); | |||
| return CNNNetwork(ngFunc); | |||
| return CNNNetwork(ngFunc, exts); | |||
There was a problem hiding this comment.
Why do we need this change?
There was a problem hiding this comment.
If we have a scenario
- add extension
- read an ONNX model
- serialize it to xml
- read the model again as xml
Passingextsis needed. It is tested byCustomOpUser_ONNXImporterunit test
| @@ -49,3 +85,63 @@ std::shared_ptr<ngraph::Function> FrontEndONNX::decode(InputModel::Ptr model) co | |||
| NGRAPH_CHECK(model_onnx != nullptr, "Invalid input model"); | |||
| return model_onnx->decode(); | |||
| } | |||
|
|
|||
| std::string FrontEndONNX::get_name() const { | |||
| return "onnx_experimental"; | |||
There was a problem hiding this comment.
right, we can change it to "onnx", but onnx frontend is still skipped in mo.py
GlebKazantaev
approved these changes
Aug 26, 2021
postrational
approved these changes
Aug 26, 2021
ilyachur
reviewed
Aug 26, 2021
Comment on lines
+99
to
+102
| static ngraph::frontend::FrontEndManager* get_frontend_manager() { | ||
| static ngraph::frontend::FrontEndManager manager; | ||
| return &manager; | ||
| } |
There was a problem hiding this comment.
Suggested change
| static ngraph::frontend::FrontEndManager* get_frontend_manager() { | |
| static ngraph::frontend::FrontEndManager manager; | |
| return &manager; | |
| } | |
| static ngraph::frontend::FrontEndManager& get_frontend_manager() { | |
| static ngraph::frontend::FrontEndManager manager; | |
| return manager; | |
| } |
There was a problem hiding this comment.
discussed offline to resolve it separately
|
Please resolve comments in the separate PR |
rnugmanx
pushed a commit
to rnugmanx/openvino
that referenced
this pull request
Aug 26, 2021
* added get_name * add support to supported_impl * remove debug code * review remarks * changed name to onnx_experimental * fixed test * revert onnx_experimental name * integrate reader and fe api * add unit tests * removed prototxt from model_validator * reader refactor * add supress * Update inference-engine/src/readers/onnx_reader/ie_onnx_reader.cpp Co-authored-by: Tomasz Dołbniak <[email protected]> * fix segfaults * removed onnx reader * handle istringstream * wstring support * removed saving path * styles applied * changed name to onnx experimental * Apply suggestions from code review Co-authored-by: Tatiana Savina <[email protected]> * skip onnx_experimental frontend in mo.py * add support of wstring paths * fix wstring ctor of InputModelONNX * added NGRAPH_SUPPRESS * make one instance of manager * change onnx_experimental name to onnx * creation frontend manager refactor Co-authored-by: Tomasz Dołbniak <[email protected]> Co-authored-by: Tatiana Savina <[email protected]>
akuporos
pushed a commit
to akuporos/openvino
that referenced
this pull request
Sep 29, 2021
* added get_name * add support to supported_impl * remove debug code * review remarks * changed name to onnx_experimental * fixed test * revert onnx_experimental name * integrate reader and fe api * add unit tests * removed prototxt from model_validator * reader refactor * add supress * Update inference-engine/src/readers/onnx_reader/ie_onnx_reader.cpp Co-authored-by: Tomasz Dołbniak <[email protected]> * fix segfaults * removed onnx reader * handle istringstream * wstring support * removed saving path * styles applied * changed name to onnx experimental * Apply suggestions from code review Co-authored-by: Tatiana Savina <[email protected]> * skip onnx_experimental frontend in mo.py * add support of wstring paths * fix wstring ctor of InputModelONNX * added NGRAPH_SUPPRESS * make one instance of manager * change onnx_experimental name to onnx * creation frontend manager refactor Co-authored-by: Tomasz Dołbniak <[email protected]> Co-authored-by: Tatiana Savina <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket: 62297, 62730
load_by_modelfor ONNX FE (from file and from stream + passing path needed for onnx external data feature)